Skip to content

GH-50774: [CI][Python] Match Protobuf symbol visibility in bundled Substrait and ORC - #50792

Merged
kou merged 7 commits into
apache:mainfrom
tadeja:50774-emscr-import-pyinit
Aug 6, 2026
Merged

GH-50774: [CI][Python] Match Protobuf symbol visibility in bundled Substrait and ORC#50792
kou merged 7 commits into
apache:mainfrom
tadeja:50774-emscr-import-pyinit

Conversation

@tadeja

@tadeja tadeja commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Rationale for this change

Fix #50774. After #50650, the Pyodide build failed while loading PyArrow with ImportError: dynamic module does not define module export function (PyInit_lib).
The ImportError is misleading because the PyInit_lib is present. The actual failure is an earlier abort during static initialization of libarrow_python.so, in Protobuf descriptor registration.

#50650 changed the order of libraries merged into libarrow_bundled_dependencies.a. The archive contains duplicate weak Protobuf/Abseil symbols emitted by the Protobuf runtime and generated code in Substrait and in ORC. These symbols had inconsistent visibility:
a) Protobuf was compiled with hidden visibility,
b) Substrait and ORC generated Protobuf code used default visibility,
so the order change caused wasm-ld to select symbols with different visibility.
(e.g. google::protobuf::internal::ZeroFieldsBase::Clear() changed from hidden in Protobuf's empty.pb.cc.o to default visible in Substrait's algebra.pb.cc.o)

The failure showed up in Emscripten, no failures on other platforms, but matching visibility globally is desirable to prevent any unintended exports and symbol interposition.

What changes are included in this PR?

Compile bundled Substrait and ORC with the same hidden visibility for symbols as Protobuf so not depending on archive order.
Also add debug for Emscripten CI builds --profiling-funcs.

Are these changes tested?

Yes, test-conda-python-emscripten passes
(Local repro, debug and fix verification with ARCH=amd64 archery docker run conda-python-emscripten, for symbols llvm-nm and wasm-ld)

Are there any user-facing changes?

No.

@tadeja

tadeja commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-conda-python-emscripten

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #50774 has been automatically assigned in GitHub to PR creator.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Revision: f5714c0

Submitted crossbow builds: ursacomputing/crossbow @ actions-50e51eced4

Task Status
test-conda-python-emscripten GitHub Actions

@tadeja

tadeja commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Although the first crossbow run failed with pre-existing script timeout at the end in the Chrome step, it actually shows problematic part now succeeds:
https://github.com/ursacomputing/crossbow/actions/runs/30844436419/job/91789163035#step:6:6200

> await pyodide.loadPackage('/build/python/dist/pyarrow-26.0.0.dev99-cp312-cp312-pyemscripten_2024_0_wasm32.whl');

Loading pyarrow
Loaded pyarrow

( While in the issue logs Failed to load pyarrow)

The log also shows the export is present in the wheel, meaning that the PyInit_lib ImportError is misleading message.

Locally I traced the cause (with fix reverted failure reproduced), symbolized by patching abort() in pyodide.asm.js to print the stack, relinked with --profiling-funcs

> await pyodide.loadPackage('/build/python/dist/pyarrow-26.0.0a0-cp312-cp312-pyemscripten_2024_0_wasm32.whl');
Loading pyarrow
ABORT:  Error
    at abort (/pyodide/pyodide.asm.js:10:6334)
    at _abort (/pyodide/pyodide.asm.js:10:289408)
    at invoke_v (/pyodide/pyodide.asm.js:10:1224993)
    at wasm://wasm/0267d6e2:wasm-function[13105]:0x531c4b
    at libarrow_python.so.google::protobuf::FileDescriptorProto::SharedDtor(google::protobuf::MessageLite&) (wasm://wasm/libarrow_python.so-0fa2ac3a:wasm-function[48283]:0x2517a8a)
    at invoke_vi (/pyodide/pyodide.asm.js:10:1223725)
    at libarrow_python.so.google::protobuf::FileDescriptorProto::~FileDescriptorProto() (wasm://wasm/libarrow_python.so-0fa2ac3a:wasm-function[48282]:0x2517795)
    at libarrow_python.so.google::protobuf::EncodedDescriptorDatabase::Add(void const*, int) (wasm://wasm/libarrow_python.so-0fa2ac3a:wasm-function[48600]:0x2527cec)
    at invoke_iiii (/pyodide/pyodide.asm.js:10:1223129)
    at libarrow_python.so.google::protobuf::internal::AddDescriptors(google::protobuf::internal::DescriptorTable const*) (wasm://wasm/libarrow_python.so-0fa2ac3a:wasm-function[48861]:0x255572e)
Aborted()
Failed to load pyarrow
The following error occurred while loading pyarrow:
Aborted(). Build with -sASSERTIONS for more info.
[]

The abort happens during static initialization of libarrow_python.so, inside protobuf's descriptor registration.

@tadeja

tadeja commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-conda-python-emscripten

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Revision: e44aaa2

Submitted crossbow builds: ursacomputing/crossbow @ actions-e7e0e9a62d

Task Status
test-conda-python-emscripten GitHub Actions

@tadeja
tadeja force-pushed the 50774-emscr-import-pyinit branch from e44aaa2 to a7d0931 Compare August 4, 2026 11:04
@tadeja

tadeja commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-conda-python-emscripten r-recheck-most

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Revision: a7d0931

Submitted crossbow builds: ursacomputing/crossbow @ actions-c8b74835fc

Task Status
r-recheck-most GitHub Actions
test-conda-python-emscripten GitHub Actions

@tadeja

tadeja commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-conda-python-emscripten

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Revision: 8d456b3

Submitted crossbow builds: ursacomputing/crossbow @ actions-6f85329ef0

Task Status
test-conda-python-emscripten GitHub Actions

@tadeja
tadeja requested a review from kou August 4, 2026 12:16
@tadeja tadeja changed the title GH-50774: [CI][Python] Reorder for cross-compiled builds GH-50774: [CI][Python] Restore bundled archive order for Emscripten Aug 4, 2026
@tadeja

tadeja commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-conda-python-emscripten

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Revision: 18130db

Submitted crossbow builds: ursacomputing/crossbow @ actions-24de2ba338

Task Status
test-conda-python-emscripten GitHub Actions

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Thanks!!!


add_library(substrait STATIC ${SUBSTRAIT_SOURCES})
set_target_properties(substrait PROPERTIES POSITION_INDEPENDENT_CODE ON)
if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this condition? (Can we always set CXX_VISIBILITY_PRESET and VISIBILITY_INLINES_HIDDEN?)

@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Aug 4, 2026
@kou

kou commented Aug 4, 2026

Copy link
Copy Markdown
Member

BTW, can we always use --profiling-funcs for easy to debug? Does it have a large performance penalty? If so, can we enable it only in our CI?

@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 5, 2026
"-DZLIB_LIBRARY=$<TARGET_FILE:ZLIB::ZLIB>")
"-DZLIB_LIBRARY=$<TARGET_FILE:ZLIB::ZLIB>"
-DCMAKE_CXX_VISIBILITY_PRESET=hidden
-DCMAKE_VISIBILITY_INLINES_HIDDEN=ON)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you sort this argument list?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh... those two lines are sorted now!

@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Aug 5, 2026
@tadeja

tadeja commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-conda-python-emscripten

@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Revision: a95ac9f

Submitted crossbow builds: ursacomputing/crossbow @ actions-578e2a8b50

Task Status
test-conda-python-emscripten GitHub Actions

@tadeja tadeja changed the title GH-50774: [CI][Python] Restore bundled archive order for Emscripten GH-50774: [CI][Python] Match Protobuf symbol visibility in bundled Substrait and ORC Aug 5, 2026
@tadeja

tadeja commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-ubuntu-22.04-cpp test-debian-13-cpp-amd64

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Revision: a95ac9f

Submitted crossbow builds: ursacomputing/crossbow @ actions-ee064bc184

Task Status
test-debian-13-cpp-amd64 GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions

@tadeja
tadeja marked this pull request as ready for review August 5, 2026 21:32
@tadeja

tadeja commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @kou , this is now ready for review

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@kou
kou merged commit d6d8f03 into apache:main Aug 6, 2026
69 of 70 checks passed
@kou kou removed the awaiting change review Awaiting change review label Aug 6, 2026
@github-actions github-actions Bot added the awaiting merge Awaiting merge label Aug 6, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit d6d8f03.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 30 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI][Python] Pyodide import error

2 participants